Support pixel density on p5.Image (fixes issue #6114) #6447

Gaurav-1306 · 2023-09-30T12:32:06Z

Resolves #6114

Changes:

Created a new method that allowed user to set up the pixel density of the image.

Screenshots of the change:

PR Checklist

npm run lint passes
Inline documentation is included / updated
Unit tests are included / updated

limzykenneth · 2023-09-30T21:00:58Z

src/image/p5.Image.js

+   */
+  setPixelDensity(density) {
+    if (density <= 0) {
+      console.error('Pixel density must be greater than 0.');


Instead of a console.error perhaps using the FES would be better here? Or alternatively default to 1 if the value is <= 0.

I have implemented both of the suggested actions for improvement. Kindly review the changes and provide any feedback. Your input is highly valued. Thank you.

davepagurek

If you call graphic.get() on a p5.Graphics (or just call get() for the main canvas), it currently returns a p5.Image at 1x density instead of matching the density of the source object. Would we be able to use this new feature here too?

p5.js/src/core/p5.Renderer.js

Lines 161 to 164 in a129d57

    
           const region = new p5.Image(w, h); 
        
           region.canvas 
        
             .getContext('2d') 
        
             .drawImage(canvas, x, y, w * pd, h * pd, 0, 0, w, h);

davepagurek · 2023-10-02T11:42:46Z

src/image/p5.Image.js

+    this.canvas.height = this.height * density;
+
+    // Update the drawing context
+    this.drawingContext = this.canvas.getContext('2d');


I think it might not be necessary to get the context again after changing the canvas size

davepagurek · 2023-10-02T11:47:30Z

src/image/p5.Image.js

+   * img.setPixelDensity(2);
+   * </code></div>
+   */
+  setPixelDensity(density) {


Do you think we could match the API of main canvases and p5.Graphics where pixelDensity() returns the current density, and pixelDensity(newDensity) sets it?

Let me look into what you are saying and get back to you. Thanks for the feedback.

Gaurav-1306 · 2023-10-04T15:07:12Z

@davepagurek I have been working on the solution.

The problem:
As you said when we call the get() function it creates an image with pixel density of 1. Now we can change the pixel desity that is hard coded for 1 to anything we like by creating a method pixelDesity() and that can easily change the pd.

now extending the problem.

if we want to use the same function for all the other canvases then I think it should be possible for us to use that as long as it an image we are talking about.

Doubt

p5.js/src/core/environment.js

Lines 679 to 686 in 95b82ea

    
           p5.prototype.pixelDensity = function(val) { 
        
             p5._validateParameters('pixelDensity', arguments); 
        
             let returnValue; 
        
             if (typeof val === 'number') { 
        
               if (val !== this._pixelDensity) { 
        
                 this._pixelDensity = val; 
        
               } 
        
               returnValue = this;

In this particular instance, the method pixelDesity already exists. I am a bit confused as to how this function links with the other function. Will the function able to change the pixel density of the image(p5.image) that we are trying to change?

davepagurek · 2023-10-04T17:24:24Z

I think we won't need to modify the pixelDensity function you linked in core/environment.js, but rather, we can add a similar function to p5.Image. We do that in p5.Framebuffer already here:

p5.js/src/webgl/p5.Framebuffer.js

Lines 268 to 276 in 95b82ea

    
           pixelDensity(density) { 
        
             if (density) { 
        
               this.autoSized = false; 
        
               this.density = density; 
        
               this._handleResize(); 
        
             } else { 
        
               return this.density; 
        
             } 
        
           }

Rather than having a separate setPixelDensity and pixelDensity for p5.Image, we could give it a combined getter/setter function like that too.

Gaurav-1306 · 2023-10-05T09:28:27Z

@davepagurek i made the changes as you said. You can provide the with feedback. one more thing, I think we can use pixeldensity function from anywhere as long as we are doing it on an image. But if we set the following function

p5.js/src/image/p5.Image.js

Line 146 in 95b82ea

this._pixelDensity = 1;

like this._pixelDensity = pixelDensity(); and then define the pixelDensity in the global scope of all the src/files then we can change the pixelDensity in the class of p5.image only.

davepagurek

But if we set the following function like this._pixelDensity = pixelDensity(); and then define the pixelDensity in the global scope of all the src/files then we can change the pixelDensity in the class of p5.image only.

I don't think we want to make image pixel densities match the display density by default, because we would be making the image's canvas more dense without having more data to actually fill it with. Say you import a 200x200 image with loadImage(). If we set its density to 2, then we will be making its canvas be 400x400, but the original image is still only 200x200, so we would just be unnecessarily stretching it to 400x400 internally.

I think it probably makes more sense if we always keep the internal image's data the same (since that basically never changes after we import an image), and instead use pixelDensity to determine how we interpret the data. So if you import a 200x200 image, but then say img.pixelDensity(2), then that would make p5 treat it like a 100x100 image, but with enough subpixels that it still displays crisply on a 2x display. This would have to be something users set manually for imported images because we wouldn't know whether an image's "target" size should be, that's up to the user and how they want to use it. @limzykenneth does that make sense to you too?

If we use it that way, then rather than adjusting the canvas size when we set the density, we would instead keep the canvas size the same and adjust the width/height p5 uses:

- this.canvas.width = this.width * density;
- this.canvas.height = this.height * density;
+ this.width /= density;
+ this.height /= density;

Can we also set the pixel density of the image returned by graphic.get()?

p5.js/src/core/p5.Renderer.js

Lines 161 to 164 in a129d57

    
           const region = new p5.Image(w, h); 
        
           region.canvas 
        
             .getContext('2d') 
        
             .drawImage(canvas, x, y, w * pd, h * pd, 0, 0, w, h);

A test case to make sure that works could be creating a graphic and setting its pixel density to e.g. 3, calling img = graphic.get(), and then checking that img.width === graphic.width, img.height === graphic.height, and img.pixelDensity() === graphic.pixelDensity().

limzykenneth · 2023-10-05T13:43:23Z

Yes, the implementation I have in my mind would more or less as @davepagurek described, since we don't want to create or delete pixels from the image loaded in by the user when pixel density is changed we'll change the dimension of the backing canvas instead. Although we probably need to be a bit aware of possible points of confusion and possibly document clearly as necessary.

Gaurav-1306 · 2023-10-05T14:46:40Z

pixelDensity to determine how we interpret the data. So if you import a 200x200 image, but then say img.pixelDensity(2), then that would make p5 treat it like a 100x100 image, but with enough subpixels that it still displays crisply on a 2x display. This would have to be something users set manually for imported images because we wouldn't know whether an image's "target" size should be, that's up to the user and how they want to use it

the above point makes sense and in the next commit I will implement so that we don't change the canvas size but rather the the interpretation of the image.

Can we also set the pixel density of the image returned by graphic.get()?

Now coming to this point, i always thought that graphic.get() function returned an image so any method on p5.image should work with this function too, but i guess i need to do testing to make sure this works. So let me read some document and get back to you.

Do you think the ideas I've shared align with what we're discussing? I'd love to hear your thoughts on it

davepagurek · 2023-10-05T14:53:36Z

Now coming to this point, i always thought that graphic.get() function returned an image so any method on p5.image should work with this function too, but i guess i need to do testing to make sure this works. So let me read some document and get back to you.

Right, I think the change would just be in how we set the default pixel density of images coming from .get(). Currently, it creates an image with less raw pixels than the source canvas if the source canvas has a density greater than 1. We'd like to make it store all the data available and match the pixel density right when you call .get() so that users don't have to then manually call .pixelDensity() on it afterwards (although, like you said, they'd be able to if they want to.)

Do you think the ideas I've shared align with what we're discussing? I'd love to hear your thoughts on it

Remind me which other ideas you're referring to? Is that the idea of having a common implementation of the pixelDensity function across p5.Image, p5.Graphics, p5.Framebuffer, and p5 itself?

Gaurav-1306 · 2023-10-05T15:00:28Z

Remind me which other ideas you're referring to. Is that the idea of having a common implementation of the pixelDensity function across p5.Image, p5.Graphics, p5.Framebuffer, and p5 itself?

Nah, I was just referring to the points I mentioned above, to be sure that we are on the same page.
Now that you have mentioned the common implementation if you could elaborate a little, like where will this pixelDensity function reside and other things that you have in mind, that would be really helpful!

davepagurek · 2023-10-05T16:02:56Z

Ah ok I thought earlier you were maybe talking about having a common implementation. I think it's OK if we don't do that for now since there are some subtle differences in how all these classes work.

Anyway other than that I think what we've been discussing seems right!

… canvas

Gaurav-1306 · 2023-10-06T10:31:19Z

@davepagurek I think i finally solved all the concern.

To begin with I change the height and width of the image canvas to not stretch internally as you suggested in the code.
the get() function - setting the image to same pixel density as that of the image the get function is called on.

p5.js/src/core/p5.Renderer.js

Line 161 in 3598675

const region = new p5.Image(w, h);

in the above line a new image is created and whenever a new image is created then the pixeldesnity of the new image is set of default of 1. To retain the same pixel density of that of the original image I add the following code in the next line.
region._pixelDensity = pd;
this makes the changes necessary for the retention of the pixel value.
following are the screenshots showing that these changes are working.

davepagurek · 2023-10-08T15:07:34Z

src/core/p5.Renderer.js

@@ -159,6 +159,7 @@ class Renderer extends p5.Element {
    }

    const region = new p5.Image(w, h);


Do we need to make it it a new p5.Image(w*pd, h*pd)? I think this currently still reduces the number of pixels if the graphic is higher density. I think we'd also need up update line 165 to be something like:

drawImage(canvas, x, y, w * pd, h * pd, 0, 0, w * pd, h * pd)

Awesome! I get what you're saying. I've made the changes in the commit. Take a look now—do you think it looks good? Super open to any further tweaks or suggestions!

davepagurek · 2023-10-08T15:08:04Z

src/image/p5.Image.js

+      this._pixelDensity = density;
+
+      // Adjust canvas dimensions based on pixel density
+      this.width /= density;


This function looks great now, thanks!

Gaurav-1306 · 2023-10-11T02:49:40Z

@davepagurek @limzykenneth
any update on this issue? Can my commits be merged?

limzykenneth · 2023-10-12T10:17:37Z

lib/empty-example/sketch.js

-  // put setup code here
+// put setup code here
 }

 function draw() {
-  // put drawing code here


Can changes in this file be reverted as it is not related to the issue being addressed?

ya let me change them.

limzykenneth · 2023-10-12T12:40:15Z

Looks good. Thanks!

fixed the issue processing#6114

c4f10ac

davepagurek self-requested a review September 30, 2023 17:31

limzykenneth reviewed Sep 30, 2023

View reviewed changes

changed error to fes system

108b3c4

davepagurek reviewed Oct 2, 2023

View reviewed changes

Gaurav-1306 added 2 commits October 5, 2023 14:47

made change to improve the pixelDensity funtion

2513351

improved pixedDensity function

dfb27a3

davepagurek reviewed Oct 5, 2023

View reviewed changes

Gaurav-1306 added 2 commits October 6, 2023 15:58

change the get() function to retain value of pd and not streching the…

66e1377

… canvas

cleaned the sketch.js

d5b5099

davepagurek reviewed Oct 8, 2023

View reviewed changes

minor changes

963b77f

Gaurav-1306 mentioned this pull request Oct 12, 2023

1.8.0 Release #6469

Closed

7 tasks

Qianqianye requested review from davepagurek and limzykenneth October 12, 2023 03:29

limzykenneth reviewed Oct 12, 2023

View reviewed changes

Gaurav-1306 added 2 commits October 12, 2023 16:08

spacing corrected

c0b719e

spacing corrected

27740b0

limzykenneth merged commit c5f5abb into processing:main Oct 12, 2023
2 checks passed

SableRaf changed the title ~~fixed the issue #6114~~ Support pixel density on p5.Image (fixes issue #6114) Oct 12, 2023

davepagurek mentioned this pull request Jan 25, 2024

Masking behaviour change between 1.4.1 and latest #6770

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support pixel density on p5.Image (fixes issue #6114) #6447

Support pixel density on p5.Image (fixes issue #6114) #6447

Gaurav-1306 commented Sep 30, 2023 •

edited

Loading

limzykenneth Sep 30, 2023

Gaurav-1306 Oct 1, 2023

davepagurek left a comment

davepagurek Oct 2, 2023

davepagurek Oct 2, 2023

Gaurav-1306 Oct 2, 2023

Gaurav-1306 commented Oct 4, 2023

davepagurek commented Oct 4, 2023

Gaurav-1306 commented Oct 5, 2023 •

edited

Loading

davepagurek left a comment

limzykenneth commented Oct 5, 2023

Gaurav-1306 commented Oct 5, 2023

davepagurek commented Oct 5, 2023

Gaurav-1306 commented Oct 5, 2023

davepagurek commented Oct 5, 2023

Gaurav-1306 commented Oct 6, 2023 •

edited

Loading

davepagurek Oct 8, 2023

Gaurav-1306 Oct 8, 2023

davepagurek Oct 8, 2023

Gaurav-1306 commented Oct 11, 2023

limzykenneth Oct 12, 2023

Gaurav-1306 Oct 12, 2023

limzykenneth commented Oct 12, 2023

	const region = new p5.Image(w, h);
	region.canvas
	.getContext('2d')
	.drawImage(canvas, x, y, w * pd, h * pd, 0, 0, w, h);

		@@ -159,6 +159,7 @@ class Renderer extends p5.Element {
		}

		const region = new p5.Image(w, h);

Support pixel density on p5.Image (fixes issue #6114) #6447

Support pixel density on p5.Image (fixes issue #6114) #6447

Conversation

Gaurav-1306 commented Sep 30, 2023 • edited Loading

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davepagurek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gaurav-1306 commented Oct 4, 2023

davepagurek commented Oct 4, 2023

Gaurav-1306 commented Oct 5, 2023 • edited Loading

davepagurek left a comment

Choose a reason for hiding this comment

limzykenneth commented Oct 5, 2023

Gaurav-1306 commented Oct 5, 2023

davepagurek commented Oct 5, 2023

Gaurav-1306 commented Oct 5, 2023

davepagurek commented Oct 5, 2023

Gaurav-1306 commented Oct 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gaurav-1306 commented Oct 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

limzykenneth commented Oct 12, 2023

Gaurav-1306 commented Sep 30, 2023 •

edited

Loading

Gaurav-1306 commented Oct 5, 2023 •

edited

Loading

Gaurav-1306 commented Oct 6, 2023 •

edited

Loading